-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrated project from pages to app router #380
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Language improvements
src/app/layout.tsx
Outdated
icon: "/favicon.ico", | ||
}, | ||
description: | ||
"Innovative and open-source visualization application that transforms various data formats, such as JSON, YAML, XML, CSV and more, into interactive graphs.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Innovative and open-source visualization application that transforms various data formats, such as JSON, YAML, XML, CSV and more, into interactive graphs.", | |
"Innovative and open-source visualization application that produces interactive graphs from various data formats, such as JSON, YAML, XML, CSV and more.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TallTed Thank you for suggesting these changes but I didn't change any language in the metatags, it was as it is from the original repo.
This PR only focuses on upgrading the project from the pages router to the app router.
Will fix these in another PR maybe.
src/app/layout.tsx
Outdated
description: | ||
"Innovative and open-source visualization application that transforms various data formats, such as JSON, YAML, XML, CSV and more, into interactive graphs.", | ||
openGraph: { | ||
title: "JSON Crack - Visualize Data to Graphs", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
title: "JSON Crack - Visualize Data to Graphs", | |
title: "JSON Crack - Visualize Data as Graphs", |
src/app/layout.tsx
Outdated
title: "JSON Crack - Visualize Data to Graphs", | ||
description: | ||
"Innovative and open-source visualization application that transforms various data formats, such as JSON, YAML, XML, CSV and more, into interactive graphs.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
title: "JSON Crack - Visualize Data to Graphs", | |
description: | |
"Innovative and open-source visualization application that transforms various data formats, such as JSON, YAML, XML, CSV and more, into interactive graphs.", | |
title: "JSON Crack - Visualize Data as Graphs", | |
description: | |
"Innovative and open-source visualization application that produces interactive graphs from various data formats, such as JSON, YAML, XML, CSV and more.", |
2332d33
to
6ea0dea
Compare
I'm not sure what you would like me to review. My previously suggested changes appear not to have been applied, though at least some of the text I suggested to change has been edited differently, but these edits are only visible commit-by-commit, not in the "files changed"... |
Hey @TallTed the changes you suggested are not in the scope of this PR, this PR only focuses on migrating the project from the pages router to the app router. Hence, I migrated the metadata from independent tags to a default object export. |
Co-authored-by: Ted Thibodeau Jr <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm fwiw (I code almost exclusively in English)
Looks like no migration made for useSearchParams, can we do that? Also why did we add the "impl" package to the project? |
@@ -36,14 +36,14 @@ const StyledHighlight = styled.span<{ $link?: boolean; $alert?: boolean }>` | |||
margin: ${({ $alert }) => ($alert ? "8px 0" : "1px")}; | |||
`; | |||
|
|||
const Docs = () => { | |||
export default function Page() { | |||
return ( | |||
<Layout> | |||
<Head> | |||
<title>Documentation - JSON Crack</title> | |||
<meta name="description" content="Embedding JSON Crack tutorial into your websites." /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<meta name="description" content="Embedding JSON Crack tutorial into your websites." /> | |
<meta name="description" content="Embedding JSON Crack tutorial in your websites." /> |
Page you are trying to open does not exist. You may have mistyped the address, or the page | ||
has been moved to another URL. If you think this is an error contact support. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Page you are trying to open does not exist. You may have mistyped the address, or the page | |
has been moved to another URL. If you think this is an error contact support. | |
The page you are trying to open does not exist. You may have mistyped the address, or the page | |
may have been moved to another URL. If you think this is an error, contact support. |
Hey @AykutSarac Thanks for the review, I have used useSearchParams from the next/navigation package which is supported in the app router, let me know why you think I should migrate it. |
It's recommended by Next.js, also fails the build. |
Okay, got it. So adding a suspense boundary to the component would suffice right? |
Objective
This PR will migrate the jsoncrack project from the old next.js pages router to the new app router.
Changes
app
directory as the entry point and move all the existing routes from thepages
directory.pages/_app.tsx
andpages/_document.tsx
files are replaced byapp/layout.tsx
.metadata
constant from theapp/layout.tsx
file. I will continue doing the same for all files.pages/_error.tsx
is replaced byapp/not-found.tsx
which can not only serve a custom error UI but can also send HTTP status and serve metatags out of the box.next/router
is replaced bynext/navigation
which is a simplified API.References used for migration
Testing
Tested the routes and functionalities by doing feature by feature test taking the production(jsoncrack.com) as the reference.